Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added multiple database support. #340

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

baudhuina
Copy link

This pull request is the result of my investigation discussion #325 . It revealed several issues when using multiple database and extensive tests have been added to detect them, and check they are properly fixed.

Changes:

  • Now include database alias in cache key to avoid cache overlap when using multiple databases.
  • Now properly support multiple databases and translatable model duplication. See multiple_db.rst for details and test_multiple_db.py for information about the tests that used to fail with v2.3.
  • Now raise exceptions when attempting a call to save() or delete() which would result in an existing model to be overwritten in the target database, since overwriting the master model without carefully processing the translation can results in data corruption (see multiple_db.rst for details).
  • Updated documentation about all changes, and included an overview of how save() now manages model creation, update, duplication and overwriting, in every single- and multi-database configuration (see multiple_db.rst).
  • Upgraded versions in docs/_ext/djangodummy/requirements.txt to use up-to-date version of Sphinx and django. Fixed all deprecation warnings.

Backward compatibility: complete.

  • All existing tests are unchanged to ensure backward compatibility, except for 2 minor mandatory changes to accommodate the new signature of get_translation_cache_key().
  • There is no change in the original code of TranslatableModelMixin.save(), TranslatableModelMixin.save_translations() and translation.save() method except for the detection of the special (and rare) situations demanding either to raise an exception or to proceed differently. There is consequently no performance impact on the most common and performance-critical operations.

Test:

  • Added a bunch of tests to first identify the issues with version 2.3 and cover all enhancements.
  • Upgraded tox.ini and tests.yaml to cover python 3.10 and django up to 4.2 and to use coverage v7.3.0
  • .coveragerc modified: now excludes tests and example.
  • I couldn't run tox locally without a change in setup.py to avoid the exception on the failed import of django on line 14. This change was NOT committed, since I have to assume I missed something in my local configuration, but I couldn't figure out was: by the time setup.py is imported, django is not installed in the venv yet by tox , and the failure seems unavoidable to me. Tests are nevertheless OK without this change when run by the CI Workflow.
  • With the above change, Tox was run locally (but python 3.6/3.7 tests were skipped, since those version cannot easily be installed on a Mac with M1 chip) and all tests succeed in the CI Workflow

Version and date

Version and date left open as 2.x in:

  • CHANGE.rst
  • multiple_db.rst
  • cache.rst
  • cache_design.rst
  • conf.py

Improved documentation in cache.py
- Corrected typo in djangodummy/settings.py: I guess 'parser' was supposed to be 'parler'.
- Added 2 pages to cover the use of multiple databases and keep track of the design decisions.
- Added new entry in changelog (no version, no date).
…elease).

- Adapted test_query_count.py in two places to accommodate the new signature of get_translation_cache_key() and new structure of the keys.
- Ran and finalized tests: all 99 OK.
…mpatibility.

Created docs/_static folder with dummy static file to silence Sphinx warning, which is treated as error when run by tox
Completed tox.ini to cover django up to 4.2 and python up to 3.10
- Added Sphinx in docs/_ext/djangodummy/requirements.txt and upgraded django, and sphinxcontrib-django to current versions
- Fixed all problems resulting in warnings during doc build with the current version of Sphinx (7.2.5) (tox upgrades them to errors).
	- Orphan page not tagged as :orphan:
	- :django:setting: role must just be :setting:
	- A couple of docstring formatting issues.
   - upgraded python and coverage versions in tox.ini
   - excluded tests and example from coverage measurement. Now down to 81%...
Added workflow_dispatch to be able to run manually
Temporarily added multiDB_support branch to have the workflow run once automatically;.
Changed platform to ubuntu-20.04 because python 3.6 is not available on ubuntu-22.04
…future__ import annotations' for pre-3.10 python, but this import is not available for 3.6.
In test.yaml, removed the trigger on branch multipleDB_support.
…ch was treated as error by tox.

- Added some additional tests in test_multiple_db() to cover create_translation()/delete_translation() methods.
…n attempt to overwrite a model in another DB. This is very unlikely to be the intention, so information about the current and target db is essential for the use to correct his use of parler.
…ved_pk which caused erroneous detections of object duplication.

- Refined tests  for detecting duplication when saving
- Refined tests accordingly.
- Added a warning in documentation: forcing pk is possible for translatable models, just as for plain models, but is equally dangerous if the use does not manage transactions properly to avoid race conditions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant